Skip to content

test(chat): NG0956 streaming regression guard (closes F6 gap)#723

Merged
blove merged 1 commit into
mainfrom
claude/f6-ng0956-guard
Jun 20, 2026
Merged

test(chat): NG0956 streaming regression guard (closes F6 gap)#723
blove merged 1 commit into
mainfrom
claude/f6-ng0956-guard

Conversation

@blove

@blove blove commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes the last open audit item — F6's missing regression guard. NG0956 ("tracking expression caused re-creation of the DOM structure") was fixed by switching the streaming-markdown @fors to track $index, but nothing protected that fix: the only coverage was the aimock e2e, which structurally can't fire NG0956 (it replays a message atomically, so the @fors render once and never re-materialize across change-detection cycles — an e2e console-guard there false-passes).

This adds a component-level vitest that drives the re-materialization directly and asserts no NG0956. Test-only, no production change.

What it does

  • Negative control — a fixed-length @for tracked by churning object identity; asserts NG0956 is captured. This proves the warning is observable in the vitest/jsdom env (the actual trigger turned out to be a fixed-length collection re-materialized with fresh identities each cycle — not a growing/reordering list).
  • Positive guards — mounts the real ChatStreamingMdComponent, feeds successive fixed-structure content (a 3-item list, then a 3-row table, with changing text) so markdown-children / markdown-table @fors genuinely re-parse to fresh token objects, and asserts no NG0956.

Validation

  • All 3 tests pass; full chat test+lint+build green.
  • Proven to catch the regression: temporarily flipping markdown-children's @for from track $index to track child (identity) makes both positive guards fail with the real NG0956 message; reverting restores green. So the guard exercises the right code and isn't a hollow pass.

🤖 Generated with Claude Code

@blove
test(chat): NG0956 streaming regression guard (F6 close-out)
0e5a68c 
A component-level vitest that re-materializes the streaming-markdown @for
collections (markdown-children list + markdown-table) across change-detection
cycles and asserts no NG0956 — the regression class the atomic aimock e2e
replay structurally cannot catch. Includes a negative control proving NG0956
is observable in this env; validated to fail if the @for reverts to
track-by-identity. Test-only; closes the F6 NG0956 guard gap.
@vercel

vercel Bot commented Jun 20, 2026 •
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
threadplane Ready Ready Preview, Comment Jun 20, 2026 5:19am

Request Review

@blove blove enabled auto-merge (squash) June 20, 2026 05:17
github-actions[bot]
github-actions Bot approved these changes Jun 20, 2026
View reviewed changes

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated approval: this PR received an intelligent (AI) code review. See the review comments on this PR.

@github-actions

github-actions Bot commented Jun 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Claude finished @blove's task in 2m 7s —— View job

PR Review

  • Gather context
  • Read changed file and production components under test
  • Review for correctness, coverage, and Angular pitfalls
  • Post findings

Summary

Test-only PR adding a vitest component-level regression guard for NG0956 in streaming markdown. No production code changes. The test strategy is sound and the guard is well-constructed. LGTM.

What the tests cover

Test What it exercises Component @for
Negative control IdentityTrackHost — track item (identity) on churning refs Proves NG0956 is observable in jsdom
List guard ChatStreamingMdComponent → MarkdownChildrenComponent track $index (line 31)
Table guard ChatStreamingMdComponent → MarkdownChildrenComponent → MarkdownTableComponent track $index (line 20)

All @fors in the streaming markdown path that previously triggered NG0956 are covered. The grep confirms no other @for in libs/chat/src uses identity-based tracking.

Review notes

Negative control design (streaming-markdown.ng0956.spec.ts:66–82) — Correct approach. The IdentityTrackHost starts with an empty signal and calls rematerialize() 4 times. The first cycle transitions 0→3 items (doesn't trigger NG0956), cycles 2–4 are same-length with all-new object refs (triggers NG0956). cap.hits.length > 0 is the right assertion here.

sink as never cast (line 33–34) — Slightly unusual but acceptable for vitest test code; MockInstance's mockImplementation overloads don't accept (...args: unknown[]) => void directly. No concern.

Spy cleanup via try/finally — Correct. Restores console.warn/console.error even if an assertion throws. Covers both channels since Angular emits NG0956 via console.error in dev mode (Angular 16+).

No explicit TestBed.resetTestingModule() in afterEach — Not an issue. Angular's TestBed resets itself when configureTestingModule is called again in the next beforeEach. The two describe blocks configure different modules; the second beforeEach cleanly overrides the first.

streaming signal is never set to false — Intentional; this guard targets the streaming re-parse path specifically. The finalization path is a separate concern.

One observation: The table test exercises MarkdownTableComponent's @for (row of bodyRows(); track $index), but bodyRows() returns rows.slice(1) (body rows, excluding header). With the 3-row body (alpha/beta/gamma), the same track $index fix is exercised on each re-parse. Correct coverage.

No bugs, no Angular/RxJS pitfalls, no type-safety issues found. The guard is proven to catch the regression (temporarily flipping to track child breaks both positive guards per the PR description).

@blove blove merged commit a8b0d91 into main Jun 20, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@blove